[codex] Make Briefprint explicit-use#20
Conversation
|
Self-review summary:
Remaining caveat: actual model-side skill invocation still depends on host telemetry and is tracked as a manual benchmark worksheet with |
There was a problem hiding this comment.
Code Review
This pull request transitions the Briefprint skill to an explicit-use model, disabling implicit model invocation and replacing broad natural-language triggers with explicit command invocations (such as $briefprint or /briefprint). It updates the documentation, metadata, evaluation cases, validation scripts, and tests to enforce this boundary and clarify the separation between the static skill bundle and the runtime cache. Feedback on the changes includes a recommendation to refine the regular expression used for detecting explicit invocations in scripts/validate_skill.py to prevent false positives on file paths or URLs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _has_explicit_briefprint_invocation(prompt: str) -> bool: | ||
| return re.search(r"(?<![\w-])(?:\$briefprint|/briefprint|briefprint)(?![\w-])", prompt.lower()) is not None |
There was a problem hiding this comment.
The current regular expression for detecting explicit briefprint invocations is susceptible to false positives when the prompt contains repository file paths (e.g., skills/briefprint/SKILL.md) or URLs (e.g., https://github.com/dd3ok/briefprint). This is because / is not included in the character class of the lookbehind and lookahead assertions, allowing briefprint to match even when it is part of a path or URL.
Including / in the lookbehind and lookahead character classes ([\w/-]) ensures that briefprint is only matched as a standalone command/invocation and not as part of a file path or URL.
| def _has_explicit_briefprint_invocation(prompt: str) -> bool: | |
| return re.search(r"(?<![\w-])(?:\$briefprint|/briefprint|briefprint)(?![\w-])", prompt.lower()) is not None | |
| def _has_explicit_briefprint_invocation(prompt: str) -> bool: | |
| return re.search(r"(?<![\w/-])(?:\$briefprint|/briefprint|briefprint)(?![\w/-])", prompt.lower()) is not None |
Summary
$briefprintfor Codex,/briefprintfor Claude Code) and reject non-explicit positive trigger fixtures.cache/briefprint, 7d document TTL guidance, no background cleanup daemon, and static skill bundle vs runtime cache separationResearch basis
SKILL.mdbundles with optional scripts/references/assets.disable-model-invocation: truefor manual-only skills and/skill-nameinvocation.Validation
python -m pytest -q-> 168 passed, 2 skippedpython -m pytest -q -p no:cacheprovider-> 168 passed, 2 skippedpython scripts\validate_skill.py --run-evals-> OK, 24 test files, 9 eval cases, 10 trigger cases, 5 model benchmark casesruff check .-> All checks passedgit diff --check/ staged diff check -> no whitespace errors; Git only reported CRLF conversion warningsReview
VALIDATION.mdreceipt updateRemaining caveat
observed_invocation: null), not something CI can prove across Codex, Claude Code, Gemini, or other hosts.